Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support Worker API on mingw if winpthread is present #440

Closed
wants to merge 1 commit into from

Conversation

andrjohns
Copy link
Contributor

This PR enables the Worker API on Windows if the winpthreads library is available in mingw (usually installed by default as a dependency of gcc/clang), which is already checked as part of cmake's find_package(Threads) call.

The only part that I'm unsure about is that mingw does not have a native pipe() function - so I've wrapped the Windows _pipe() for compatibility

@saghul
Copy link
Contributor

saghul commented Jun 24, 2024

Not a fan of winpthreads. I'd rather add native threads compatibility, like we did for other locking primitives to unlock cross platform Atomics.

That would also work on Windows with MSVC for example.

@andrjohns
Copy link
Contributor Author

Not a fan of winpthreads. I

Agreed, but this probably shouldn't be framed as being about winpthreads specifically - more that the behaviour should be enabled based on feature detection instead of OS

How about I change this to cmake setting HAS_PTHREAD if POSIX threads available, and then the Worker API is enabled conditional on that?

@saghul
Copy link
Contributor

saghul commented Jun 26, 2024

It's not pthread specific, I just don't want to have 2 separate implementations on Windows.

@andrjohns
Copy link
Contributor Author

Fair enough

@andrjohns andrjohns closed this Jun 26, 2024
@saghul
Copy link
Contributor

saghul commented Jun 26, 2024

Thanks for understanding 🙏

If you want to take a look, my plan is to port the thread implementation from libuv, like the locks and such.

Then port the worker API to use that new js_thread_* family of APIs.

Having the 262 runner ported is a separate goal IMHO, since it uses more Unix APIs we'd need to add.

@andrjohns andrjohns deleted the mingw-pthreads branch July 1, 2024 13:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants